-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create ArrayScalarBuilder
for creating single element List arrays
#13623
Conversation
be70e93
to
26f2492
Compare
@@ -321,89 +321,180 @@ pub fn longest_consecutive_prefix<T: Borrow<usize>>( | |||
count | |||
} | |||
|
|||
/// Creates single element [`ListArray`], [`LargeListArray`] and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to pull the wrapping code into this structure so it remains consistent across list types
datafusion/common/src/utils/mod.rs
Outdated
/// assert_eq!(list_arr.len(), 1); | ||
/// ``` | ||
#[derive(Debug, Clone)] | ||
pub struct SingleRowArrayBuilder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems this builder is for List, maybe name with SingleRowListArrayBuilder
is more concise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 505edc9 👍
Thanks @alamb |
Thank you @jayzhan211 |
Which issue does this PR close?
Rationale for this change
There are a bewildering number of small free functions for performing the same basic thing: converting an array into a single row list array / scalar
This has also lead to subtle bugs such as #13481
Let's encapsulate that operation into a well documented structure
What changes are included in this PR?
ArrayScalarBuilder
Are these changes tested?
Are there any user-facing changes?